Skip to content

Conversation

benma
Copy link
Collaborator

@benma benma commented Aug 13, 2025

End-goal: reduce the number of secure chip ops when signing a BTC transaction, to reduce the chance of going over the Optiga chip's "rate limit", which induces throttling.

By default keystore::get_xpub computed the xpub twice, to mitigate potential bitflips, which could be bad when delivering the wrong xpub (or derivatives) to the host.

When signing a transaction however, one does not need the extra protection - if there is a bit flip, the resulting signature will be invalid.

This commit reduces the number of secure chip ops needed when the bitflip mitigation is not required.

The existing method get_xpub was renamed so the compiler can tell us all the instances where we need to decide between one or the other.

@benma benma requested a review from Beerosagos August 13, 2025 13:30
PHAMVANPHUHP

This comment was marked as spam.

Copy link
Collaborator

@Beerosagos Beerosagos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK, with a small suggestion


/// Derives an xpub from the root xpub using the provided keypath.
fn from_keypath(keypath: &[u32]) -> Result<Self, ()>;
fn from_keypath(keypath: &[u32], compute: Compute) -> Result<Self, ()>;
}

/// Implements a cache for xpubs. Cached intermediate xpubs are used to derive child xpubs.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the change, but I think that the cache_keypath() in the comment should be changed to add_keypath()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added separate commit.

@@ -17,12 +17,18 @@ use super::keystore;
use crate::bip32;
use alloc::vec::Vec;

#[derive(Copy, Clone)]
pub enum Compute {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Maybe it could be useful to add a comment here (or somewhere else) explaining that this is used as an additional bitflips check for sensitive operations

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments.

benma added 2 commits August 19, 2025 20:37
End-goal: reduce the number of secure chip ops when signing a BTC
transaction, to reduce the chance of going over the Optiga chip's
"rate limit", which induces throttling.

By default keystore::get_xpub computed the xpub twice, to mitigate
potential bitflips, which could be bad when delivering the wrong
xpub (or derivatives) to the host.

When signing a transaction however, one does not need the extra
protection - if there is a bit flip, the resulting signature will be
invalid.

This commit reduces the number of secure chip ops needed when the
bitflip mitigation is not required.

The existing method `get_xpub` was renamed so the compiler can tell us
all the instances where we need to decide between one or the other.
@benma benma merged commit 43c9a23 into BitBoxSwiss:master Aug 20, 2025
32 checks passed
@benma benma deleted the xpub-once branch August 20, 2025 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants